Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix duplicate copyOnSelect when clicking on an unfocused terminal #4596

Merged
2 commits merged into from
Feb 20, 2020

Conversation

leonMSFT
Copy link
Contributor

@leonMSFT leonMSFT commented Feb 14, 2020

Summary of the Pull Request

Currently, clicking on an unfocused terminal with a selection active will trigger copyOnSelect. This is because the check for copyOnSelect and copying to the clipboard is bound to when the Pointer is released. This works fine for when a user performs a click-drag selection, but it inadvertently also triggers when the user performs a single click on an unfocused terminal. We expect copyOnSelect to trigger only on the first time a selection is completed.

This PR will allow the user to single click on an unfocused terminal that has a selection active without triggering a copyOnSelect. It also ensures that any click-drag selection, whether it's on an unfocused or focused terminal, will trigger copyOnSelect.

PR Checklist

Validation Steps Performed

Performed manual testing involving permutations of multiple panes, tabs, in focus, and out of focus.

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm starting to wonder if any of the Terminal logic for handling a single cell selection is even necessary.

src/cascadia/TerminalControl/TermControl.h Outdated Show resolved Hide resolved
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate that this fixes the issue, but we're going to have to deeply consider how focus-raise, mouse move and copy-on-select all work together so we get to stop band-aid-ing it.

@DHowett-MSFT DHowett-MSFT added the AutoMerge Marked for automatic merge by the bot when requirements are met label Feb 20, 2020
@ghost
Copy link

ghost commented Feb 20, 2020

Hello @DHowett-MSFT!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 2dec894 into master Feb 20, 2020
@ghost ghost deleted the dev/lelian/copyonselectdupe branch February 20, 2020 18:30
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoMerge Marked for automatic merge by the bot when requirements are met
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate copyOnSelect actions
3 participants